Skip to content

fix: use rm -f to handle missing .py files in llamacpp install#932

Merged
ericcurtin merged 1 commit into
mainfrom
fix-e2e-macos-rm-py
May 21, 2026
Merged

fix: use rm -f to handle missing .py files in llamacpp install#932
ericcurtin merged 1 commit into
mainfrom
fix-e2e-macos-rm-py

Conversation

@ericcurtin
Copy link
Copy Markdown
Contributor

Summary

  • The macOS e2e test has been failing across all PRs because rm install/bin/*.py exits non-zero when no .py files exist
  • llama.cpp no longer ships Python scripts in install/bin/, so the glob expands to nothing and bare rm fails
  • Changed rm to rm -f so the cleanup step silently succeeds when no files match

Root cause

rm: install/bin/*.py: No such file or directory
make[2]: *** [build] Error 1

rm -f is the idiomatic fix — it ignores nonexistent files without changing behavior when files do exist.

llama.cpp no longer ships Python scripts in install/bin/, so the
glob expands to nothing and bare rm exits non-zero, failing the
macOS e2e build. Using rm -f silently succeeds when no files match.
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the llamacpp/Makefile to use the -f flag with the rm command when cleaning up Python scripts in the installation directory. A review comment suggests quoting the installation directory path to ensure the command handles directory names with spaces correctly, improving the robustness of the build process.

Comment thread llamacpp/Makefile
--prefix $(INSTALL_DIR)
@echo "Cleaning install directory..."
rm $(INSTALL_DIR)/bin/*.py
rm -f $(INSTALL_DIR)/bin/*.py
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The path should be quoted to handle cases where $(INSTALL_DIR) contains spaces. Unquoted paths in shell commands can lead to incorrect file deletion or command failure if the directory name contains whitespace. This aligns with the repository's focus on correctness and safety (Repository Style Guide, line 24).

	rm -f "$(INSTALL_DIR)/bin/"*.py
References
  1. The repository style guide prioritizes correctness and safety. Unquoted paths in shell commands are a common source of bugs and safety issues when directory names contain spaces. (link)

@ericcurtin ericcurtin merged commit fc41f13 into main May 21, 2026
14 checks passed
@ericcurtin ericcurtin deleted the fix-e2e-macos-rm-py branch May 21, 2026 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants